Skip to content

[Mirror] Fix assert_defined#78

Open
csiefer2 wants to merge 7 commits intodevelopfrom
pr-mirror-15114
Open

[Mirror] Fix assert_defined#78
csiefer2 wants to merge 7 commits intodevelopfrom
pr-mirror-15114

Conversation

@csiefer2
Copy link
Copy Markdown
Owner

@csiefer2 csiefer2 commented Apr 9, 2026

Automated mirror of upstream PR trilinos#15114 ## Motivation
It looks like assert_defined only asserts that the first variable is defined..

cgcgcg added 7 commits April 8, 2026 21:28
Signed-off-by: Christian Glusa <caglusa@sandia.gov>
Signed-off-by: Christian Glusa <caglusa@sandia.gov>
Signed-off-by: Christian Glusa <caglusa@sandia.gov>
Signed-off-by: Christian Glusa <caglusa@sandia.gov>
Signed-off-by: Christian Glusa <caglusa@sandia.gov>
Signed-off-by: Christian Glusa <caglusa@sandia.gov>
Signed-off-by: Christian Glusa <caglusa@sandia.gov>
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors ASSERT_DEFINED calls across numerous CMakeLists.txt files to assert variables individually rather than passing multiple arguments to a single call. This pattern is applied throughout various packages such as MueLu, Stokhos, and Thyra. Feedback indicates that in packages/nox/test/epetra/Thyra/CMakeLists.txt, some calls still contain multiple variables, which should be split into individual assertions to ensure all variables are properly checked.

Comment on lines +8 to +9
ASSERT_DEFINED(NOX_ENABLE_Epetra NOX_ENABLE_EpetraExt)
ASSERT_DEFINED(NOX_ENABLE_Stratimikos Stratimikos_ENABLE_AztecOO)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The pull request description states that ASSERT_DEFINED only asserts the first variable. However, these lines still contain multiple arguments, meaning NOX_ENABLE_EpetraExt and Stratimikos_ENABLE_AztecOO will not be checked. These should be split into individual calls to ensure all variables are properly asserted.

ASSERT_DEFINED(NOX_ENABLE_Epetra)
ASSERT_DEFINED(NOX_ENABLE_EpetraExt)
ASSERT_DEFINED(NOX_ENABLE_Stratimikos)
ASSERT_DEFINED(Stratimikos_ENABLE_AztecOO)

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 9, 2026

CDash for AT1 results [Only accessible from Sandia networks]
CDash for AT2 results [Currently only accessible from Sandia networks]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants